added tests#1024
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I’m requesting changes because the test suite is missing critical assertions and required example cases, which are part of the task’s core requirements and block completion.
Critical issues to fix:
- Add tests for the required examples:
splitInteger(17, 4)andsplitInteger(32, 6)(src/splitInteger.test.js). Assert either exact arrays or the required properties. - For each main test (including
splitInteger(8, 1)andsplitInteger(6, 2)), also assert the general properties: length equalsnumberOfParts, every element is an integer (Number.isInteger), the sum equalsvalue, andMath.max(...parts) - Math.min(...parts) <= 1. - Update the sorting test to use an input that produces unequal parts (e.g.,
splitInteger(17, 4)orsplitInteger(32, 6)) so it actually validates ascending order. - For the
value < numberOfPartscase, assert sum equals the original value, all elements are integers, the array is sorted ascending, andmax - min <= 1.
Recommendation: create a small helper in the test file (e.g., assertSplitProperties(value, numberOfParts)) to run the common checks (length, integers, sorted ascending, sum, max-min <= 1) and call it from each test to avoid duplication. The implementation looks solid—once these tests are added, you should be good to go. Keep up the good work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| @@ -1,21 +1,28 @@ | |||
| 'use strict'; | |||
| "use strict"; | |||
There was a problem hiding this comment.
The test suite is missing the required example cases splitInteger(17, 4) and splitInteger(32, 6) which the task description expects to be tested. Add tests that assert those exact behaviors (or the required properties for those inputs). The description lists the examples that must be satisfied: splitInteger(8,1), splitInteger(6,2), splitInteger(17,4), and splitInteger(32,6) .
| test(`should split a number into equal parts | ||
| if a value is divisible by a numberOfParts`, () => { | ||
|
|
||
| expect(splitInteger(6, 2)).toEqual([3, 3]); |
There was a problem hiding this comment.
This test only asserts equality for the divisible case (expect(splitInteger(6, 2)).toEqual([3, 3])). It should also assert the general properties required by the task: the returned array has length numberOfParts, every element is an integer (use Number.isInteger), the sum of elements equals the input value, and Math.max(...parts) - Math.min(...parts) <= 1. Add those assertions here or in a helper used across tests.
| test("should sort parts ascending if they are not equal", () => { | ||
| const result = splitInteger(6, 2); | ||
|
|
||
| for (let i = 0; i < result.length - 1; i++) { | ||
| expect(result[i + 1]).toBeGreaterThanOrEqual(result[i]); | ||
| } |
There was a problem hiding this comment.
The sorting test uses splitInteger(6, 2) which returns equal elements ([3,3]), so the loop doesn’t validate ordering for unequal parts. Use an input that produces differing parts (for example splitInteger(17, 4) or splitInteger(32, 6)) to meaningfully test that the returned array is sorted ascending.
| test(`should return a part equals to a value | ||
| when splitting into 1 part`, () => { | ||
|
|
||
| expect(splitInteger(8, 1)).toEqual([8]); |
There was a problem hiding this comment.
The single-part test expect(splitInteger(8, 1)).toEqual([8]) is fine as a specific example, but it should also assert the general properties (length, integer elements, sum equals 8, and max - min <= 1). Include those assertions so the test suite uniformly verifies the required properties across cases.
| const result = splitInteger(5, 6); | ||
|
|
||
| expect(result).toContain(0); | ||
| expect(result).toHaveLength(6); |
There was a problem hiding this comment.
The value < numberOfParts test checks only that a 0 is present and the length is correct. Instead (or in addition) assert that the array sums to 5, that all items are integers, that the array is sorted ascending, and that max - min <= 1 so this case also satisfies the task's general requirements.
| "use strict"; | ||
|
|
||
| const splitInteger = require('./splitInteger'); | ||
| const splitInteger = require("./splitInteger"); |
There was a problem hiding this comment.
Consider adding a small helper function in this test file (e.g., assertSplitProperties(value, numberOfParts)) that runs the common assertions: length equals numberOfParts, all elements are integers, array is sorted ascending, sum equals value, and max - min <= 1. Call it for the required examples and edge cases to avoid duplicated code and ensure consistency across tests.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Decision: APPROVE
Your solution meets the core requirements: the function consistently returns exactly numberOfParts integers whose sum equals value, the array is non-decreasing, and the max-min difference is at most 1. It also produces the specified example outputs (e.g., splitInteger(17, 4) => [4,4,4,5], splitInteger(32, 6) => [5,5,5,5,6,6]) and correctly handles cases where numberOfParts > value by including zeros. The test suite is clear, reusable via a helper, and verifies both the general properties and the example cases.
Minor suggestions (non-blocking):
- Consider tightening the test for
numberOfParts > valueto assert the exact expected distribution rather than just checking.toContain(0). - Add a brief comment in the implementation describing the floor-and-distribute approach for readability.
Nice work—clean, concise, and fully compliant with the task requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.